interface for changing bitrate by cperkulator · Pull Request #1030 · hardbyte/python-can

cperkulator · 2021-04-23T22:31:21Z

Quick implementation of changing the bitrate at runtime. I just want to get a feel if this is the interface we would like or if there is something better.

I just implemented the Vector interface for now but I can add socketcan eventually (would close #549 ).

This might be a start for implementing autobaud as well.

TODO

need to add error handling

implemented in vector. will add to socketcan eventually. just want to get some thoughts on this interface

codecov · 2021-04-23T22:33:21Z

Codecov Report

Merging #1030 (48b8f8c) into develop (7389b23) will decrease coverage by 2.12%.
The diff coverage is 39.21%.

@@             Coverage Diff             @@
##           develop    #1030      +/-   ##
===========================================
- Coverage    66.51%   64.38%   -2.13%     
===========================================
  Files           78       79       +1     
  Lines         7561     7651      +90     
===========================================
- Hits          5029     4926     -103     
- Misses        2532     2725     +193

felixdivo · 2021-04-26T09:06:26Z

Wouldn't it be nice to implement a more generic change_parameters() method that takes any parameters (just like on bus init)? The method can then complain whether that specific setting is supported to be changed or not.

felixdivo · 2021-04-26T16:53:36Z

can/interfaces/vector/canlib.py


+    @property
+    def bitrate(self) -> int:
+        return self.bitrate


Doesn't this recuse infinitely? Usually, I see this being implemented with a private attribute self._bitrate.

oops yeah I'll get that fixed. Just tossed this together quick to get some feedback.

felixdivo · 2021-04-26T16:57:01Z

Your proposal for implementing it as a property looks really nice! However, as a change to the core class of the library this should get more feedback.

(Side note: This could also be used in other places since it modularizes the configuration of the bus. Sometimes, that might make things more readable.)

zariiii9003 · 2021-04-28T14:07:46Z

Some questions:

How would you deal with CAN FD?
Regarding the Vector interface: what happens if you do not have Init Access?

felixdivo · 2021-05-11T07:39:20Z

How would you deal with CAN FD?

I guess we would not have to implement it right away. Besides, wouldn't the bitrate be handled automatically?

Regarding the Vector interface: what happens if you do not have Init Access?

I'd propose to simply throw one of the new exceptions, like a CanOperationError.

implement bitrate for socketcan using libsocketcan2. could just use ip and parse output to reduce dependency

cperkulator · 2021-05-14T16:56:32Z

How would you deal with CAN FD?

I guess we would not have to implement it right away. Besides, wouldn't the bitrate be handled automatically?

This will only set the arbitration bitrate. Would probably need a separate property for the data bitrate.

Regarding the Vector interface: what happens if you do not have Init Access?

I'd propose to simply throw one of the new exceptions, like a CanOperationError.

Check out the latest commit. I think it should handle these issues. I still need to add unit tests but testing from home this all works fine. I'll need to test on my work setup too next week.

can/interfaces/socketcan/constants.py

cperkulator · 2021-05-14T17:02:21Z

can/interfaces/socketcan/socketcan.py

 log_tx = log.getChild("tx")
 log_rx = log.getChild("rx")

+can_config_lib = ctypes.util.find_library("socketcan")


Simpler to use this library rather than parsing output from ip but adds a dependency. Thoughts?

cperkulator · 2021-05-14T17:04:26Z

can/interfaces/vector/canlib.py

        )

        if permission_mask.value == self.mask:
+            self._init_access = True


I'm not a huge fan of this flag but I would rather not check for init access everytime.

zariiii9003 · 2023-06-04T12:41:04Z

@hartkopp and @lumagi : What do you think about the socketcan part of this PR? Is that a viable solution to implement bit timing configuration for socketcan?

hartkopp · 2023-06-04T13:46:46Z

@hartkopp and @lumagi : What do you think about the socketcan part of this PR? Is that a viable solution to implement bit timing configuration for socketcan?

The bitrate setting of 'real' CAN interfaces can only be performed as network-admin (aka root) on Linux. That's why the ip tool is mostly used with sudo ip ...

And the setting affects all users on the system (not only python-can users), as the interface has to be shut down to change the bitrates (and has to be set to "up" afterwards). Shutting down the interfaces leads to -ENETDOWN return values for all user space applications working on that interface. Additionally the python-can application needs to be run as root to manipulate the interface status and the bitrate.

So reading the interface properties via ip tool or the netlink socket is fine - but setting new bitrates from python-can is probably not as feasible as suggested by the patch.

zariiii9003 · 2023-06-04T14:13:10Z

@hartkopp Thank you for your input. It's always neat to have the expert here 👍
It also seems like libsocketcan is not maintained anymore. The last commit is from 2020 and it does not support CAN FD. So it's probably not a good foundation to build upon.

hartkopp · 2023-06-04T15:13:38Z

It also seems like libsocketcan is not maintained anymore. The last commit is from 2020 and it does not support CAN FD. So it's probably not a good foundation to build upon.

@marckleinebudde : Is there any ongoing activity planned for libsocketcan ?

lumagi · 2023-06-04T17:21:27Z

I agree with @hartkopp. The executing process must be root or at least have the NET_ADMIN capability to change the interface. Neither one of those things is something you want to do to your Python interpreter on a regular basis.

marckleinebudde · 2023-06-06T07:01:15Z

There is no immediate activity planned for libsocketcan, but patches are welcome. However I'd rather have a proper (dbus?) interface for systemd-networkd to configure CAN (and networking in general).

Caleb Perkinson added 2 commits April 23, 2021 17:26

interface for changing bitrate

c2260f3

implemented in vector. will add to socketcan eventually. just want to get some thoughts on this interface

Format code with black

2faf400

mergify bot requested a review from hardbyte April 23, 2021 22:31

making it a property

786525d

cperkulator marked this pull request as draft April 24, 2021 13:24

felixdivo added enhancement api labels Apr 26, 2021

Format code with black

6a609ef

felixdivo reviewed Apr 26, 2021

View reviewed changes

cperkulator added 6 commits May 11, 2021 20:00

add init access handling and FD

f8fef99

Format code with black

4d305b2

shouldn't have made this abstract

896b915

Format code with black

48052ae

only works with python versions greater than 3.3

6d80f4c

implement bitrate for socketcan using libsocketcan2. could just use ip and parse output to reduce dependency

Format code with black

46365fd

removing debug info

48b8f8c

cperkulator commented May 14, 2021

View reviewed changes

Comments

Conversation

cperkulator commented Apr 23, 2021

Uh oh!

codecov bot commented Apr 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

felixdivo commented Apr 26, 2021

Uh oh!

felixdivo Apr 26, 2021

Choose a reason for hiding this comment

Uh oh!

cperkulator Apr 27, 2021

Choose a reason for hiding this comment

Uh oh!

felixdivo commented Apr 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zariiii9003 commented Apr 28, 2021

Uh oh!

felixdivo commented May 11, 2021

Uh oh!

cperkulator commented May 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

cperkulator May 14, 2021

Choose a reason for hiding this comment

Uh oh!

cperkulator May 14, 2021

Choose a reason for hiding this comment

Uh oh!

zariiii9003 commented Jun 4, 2023

Uh oh!

hartkopp commented Jun 4, 2023

Uh oh!

zariiii9003 commented Jun 4, 2023

Uh oh!

hartkopp commented Jun 4, 2023

Uh oh!

lumagi commented Jun 4, 2023

Uh oh!

marckleinebudde commented Jun 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

codecov bot commented Apr 23, 2021 •

edited

Loading

felixdivo commented Apr 26, 2021 •

edited

Loading

cperkulator commented May 14, 2021 •

edited

Loading

marckleinebudde commented Jun 6, 2023 •

edited

Loading